-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ice transport api to get selected pair stats #2923
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2923 +/- ##
==========================================
+ Coverage 78.92% 78.97% +0.05%
==========================================
Files 89 89
Lines 8568 8586 +18
==========================================
+ Hits 6762 6781 +19
Misses 1315 1315
+ Partials 491 490 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
3851bc2
to
b5efa6c
Compare
In use cases like SFU, it is useful to get just the selected candidate pair stats to have access to current RTT on the peer connection. The standard has a way to do `GetSelectedCandidatePair` on `ICETransport`, but does not have a way to get stats of that pair. Although not in standard, adding a method to `ICETransport` to get selected candidate pair along similar lines of above method.
b5efa6c
to
5a44fc7
Compare
if err != nil { | ||
g.log.Error(err.Error()) | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A slight change in behaviour here. Consolidate the conversion from ice.CandidatePairStats
-> webrtc.ICECandidatePairStats
in stats.go
so that it can be use both here and in getSelectedCandidatePairStats()
👇 . In fact, linter flagged it for repeated code 🎉 .
Previously, code continued to add stats after just logging the error. Now, it does not. The consolidation code in stats.go
returns an empty stats if there is an error here.
I think the state error should never happen. There is also a comment in code like
default:
// NOTE: this should never happen[tm]
err := fmt.Errorf("%w: %s", errStatsICECandidateStateInvalid, state.String())
return StatsICECandidatePairState("Unknown"), err
So, just skipping adding the stat on error.
Love how you did this @boks1971 :) Zero API break, re-used existing structures and 'feels ORTC-like'! |
Thank you @Sean-Der !!! |
In use cases like SFU, it is useful to get just the selected candidate pair stats to have access to current RTT on the peer connection. The standard has a way to do
GetSelectedCandidatePair
onICETransport
, but does not have a way to get stats of that pair.Although not in standard, adding a method to
ICETransport
to get selected candidate pair along similar lines of above method.